MSC4335: M_USER_LIMIT_EXCEEDED error code #4335
Conversation
hughns
changed the title
turt2live
added
proposal
|
@mscbot fcp merge |
|
Team member @mscbot has proposed to merge this. The next step is review by the rest of the tagged people: Concerns:
Once at least 75% of reviewers approve (and there are no outstanding concerns), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for information about what commands tagged team members can give me. |
mscbot
added
proposed-final-comment-period
| This error code does not specify the exact nature of the limit that was exceeded, which could | ||
| potentially lead to ambiguity. However, this is consistent with other Matrix error codes that | ||
| rely on the human-readable `error` field to provide specific details. Instead the `info_uri` | ||
| provides a way for the homeserver to apply arbitrary limits without the client having to understand | ||
| every type in advance. |
There was a problem hiding this comment.
Should we include a type field that gets passed to info_uri as a query parameter or something?
There was a problem hiding this comment.
Why not just encode the necessary information in info_uri straight away on the server side?
There was a problem hiding this comment.
Why not just encode the necessary information in
info_uristraight away on the server side?
Exactly. The intention is that the URIs are opaque and the homeserver may choose to put a type or similar field in there.
I've attempted to clarify this in 4dd453b.
There was a problem hiding this comment.
Why not just encode the necessary information in
info_uristraight away on the server side?
Well I assume you want to tell people what limit they've passed, would each of those be a separate info_uri? Feels like it ties your homeserver implementation tightly to whatever is at those URIs.
There was a problem hiding this comment.
I'm not understanding how just saying they're "opaque" resolves this. This makes it very tied to particular homeserver implementations.
There was a problem hiding this comment.
Screenshots are at the top of this PR. #4335 (comment)
There was a problem hiding this comment.
The client knows what operation failed so can give some context. Like, in the screenshots, it can say that "upload failed" which is useful to provide the context.
There was a problem hiding this comment.
Looking at element-hq/synapse#18876, I don't see how the user will know what limit they went over though.
There was a problem hiding this comment.
In the context of Synapse and the media upload limits feature, I imagined it could be configured with context like this:
media_upload_limits:
- time_period: 24h
max_size: 100M
msc4335_info_uri: https://example.com/quota?type=daily
msc4335_soft_limit: true
msc4335_increase_uri: https://example.com/increase-quota?type=daily
- time_period: 28d
max_size: 500M
msc4335_info_uri: https://example.com/quota?type=monthly
msc4335_soft_limit: true
msc4335_increase_uri: https://example.com/increase-quota?type=monthlyWhich would result in error response such as:
{
"errcode": "M_USER_LIMIT_EXCEEDED",
"error": "User has exceeded their upload limit",
"info_uri": "https://example.com/quota?type=daily",
"soft_limit": true,
"increase_uri": "https://example.com/increase-quota?type=daily"
}The web page at the other end can then serve up content that is specific to the limit exceeded.
Generalising this, the homeserver implementation and/or operator could also do things like:
- Make use of path: https://example.com/quota-monthly
- Fragment: https://example.com/quota#monthly
- Multiple params: https://example.com/plan?action=limit_exceeded&type=upload_limit&period=monthly
In order to implement the soft limit and option to upgrade, a Synapse operator would need to make use the get_media_upload_limits_for_user from the module API to allow certain users to have different media upload limits.
@clokep is it that you prefer the approach described at https://github.com/matrix-org/matrix-spec-proposals/blob/hughns/user-limit-exceeded/proposals/4335-user-limit-exceeded.md#add-structured-error-information? If so then could you add a "vote" or preference to that alternative instead?
If not, then I feel like I am missing something.
There was a problem hiding this comment.
OK, that sounds fine. I think that's not clear from the text that a different URI per resource is expected. And the matching implementation doesn't do this.
|
MSCs proposed for Final Comment Period (FCP) should meet the requirements outlined in the checklist prior to being accepted into the spec. This checklist is a bit long, but aims to reduce the number of follow-on MSCs after a feature lands. SCT members: please check off things you check for, and raise a concern against FCP if the checklist is incomplete. If an item doesn't apply, prefer to check it rather than remove it. Unchecking items is encouraged where applicable. Checklist:
|
github-project-automation
bot
moved this to Tracking for review
in Spec Core Team Workflow
|
@mscbot concern Checklist not complete/started (any SCT member can resolve this) |
mscbot
added
the
unresolved-concerns
turt2live
moved this from Tracking for review
to Ready for FCP ticks
in Spec Core Team Workflow
9 participants
Rendered
Implementations:
In the prototype implementation when this error code is encountered for a file upload, it is rendered as follows for a soft limit:
And like this for a hard limit:
I am employed by Element to write this MSC on behalf of the Matrix Foundation for use on the matrix.org homeserver. Hence the use of the
org.matrixnamespace on the unstable values.SCT Stuff:
MSC checklist
FCP tickyboxes